reactor: coroutinize more file related functions #2899
Conversation
thread_pool::submit() will yield, so we lose nothing by converting to a coroutine, and gain some readability.
file_stat() will yield, so we lose nothing by converting to a coroutine, and gain some readability.
thread_pool::submit() will yield, so we lose nothing by converting to a coroutine, and gain some readability.
thread_pool::submit() will yield, so we lose nothing by converting to a coroutine, and gain some readability.
thread_pool::submit() will yield, so we lose nothing by converting to a coroutine, and gain some readability.
thread_pool::submit() will yield, so we lose nothing by converting to a coroutine, and gain some readability.
thread_pool::submit() will yield, so we lose nothing by converting to a coroutine, and gain some readability.
thread_pool::submit() will yield, so we lose nothing by converting to a coroutine, and gain some readability.
thread_pool::submit() will yield, so we lose nothing by converting to a coroutine, and gain some readability.
thread_pool::submit() (or waiting for the aio backend) will yield, so we lose nothing by converting to a coroutine, and gain some readability.
There was a problem hiding this comment.
Pull Request Overview
This PR converts several file-related functions in the reactor class from using callback-based futures to coroutines (co_await/co_return), improving code readability and reducing nesting complexity.
- Eliminates nested
.then()chains in favor of linear coroutine syntax - Removes redundant
futurize_invokewrapper calls - Simplifies error handling flow in asynchronous file operations
| reactor::file_stat(std::string_view pathname_view, follow_symlink follow) noexcept { | ||
| auto pathname = sstring(pathname_view); | ||
| syscall_result_extra<struct stat> sr = co_await _thread_pool->submit<syscall_result_extra<struct stat>>( | ||
| internal::thread_pool_submit_reason::file_operation, [&] { |
There was a problem hiding this comment.
The lambda captures the pathname variable by reference, but the lambda will be executed asynchronously in a thread pool after the current function scope may have ended. This creates a dangling reference. The original code correctly captured by value with [pathname = sstring(pathname), follow].
| internal::thread_pool_submit_reason::file_operation, [&] { | |
| internal::thread_pool_submit_reason::file_operation, [pathname = std::move(pathname), follow] { |
| reactor::file_accessible(std::string_view pathname_view, access_flags flags) noexcept { | ||
| auto pathname = sstring(pathname_view); | ||
| syscall_result<int> sr = co_await _thread_pool->submit<syscall_result<int>>( | ||
| internal::thread_pool_submit_reason::file_operation, [&] { |
There was a problem hiding this comment.
The lambda captures variables by reference, but the lambda will be executed asynchronously in a thread pool. This creates dangling references to pathname and flags. The original code correctly captured by value.
| internal::thread_pool_submit_reason::file_operation, [&] { | |
| internal::thread_pool_submit_reason::file_operation, [pathname, flags] { |
| reactor::file_system_at(std::string_view pathname_view) noexcept { | ||
| auto pathname = sstring(pathname_view); | ||
| syscall_result_extra<struct statfs> sr = co_await _thread_pool->submit<syscall_result_extra<struct statfs>>( | ||
| internal::thread_pool_submit_reason::file_operation, [&] { |
There was a problem hiding this comment.
The lambda captures the pathname variable by reference, but the lambda will be executed asynchronously in a thread pool. This creates a dangling reference. The original code correctly captured by value.
| internal::thread_pool_submit_reason::file_operation, [&] { | |
| internal::thread_pool_submit_reason::file_operation, [pathname] { |
| reactor::statvfs(std::string_view pathname_view) noexcept { | ||
| auto pathname = sstring(pathname_view); | ||
| syscall_result_extra<struct statvfs> sr = co_await _thread_pool->submit<syscall_result_extra<struct statvfs>>( | ||
| internal::thread_pool_submit_reason::file_operation, [&] { |
There was a problem hiding this comment.
The lambda captures the pathname variable by reference, but the lambda will be executed asynchronously in a thread pool. This creates a dangling reference. The original code correctly captured by value.
| internal::thread_pool_submit_reason::file_operation, [&] { | |
| internal::thread_pool_submit_reason::file_operation, [pathname] { |
| auto name = sstring(name_view); | ||
| auto oflags = O_DIRECTORY | O_CLOEXEC | O_RDONLY; | ||
| syscall_result_extra<struct stat> sr = co_await _thread_pool->submit<syscall_result_extra<struct stat>>( | ||
| internal::thread_pool_submit_reason::file_operation, [&] { |
There was a problem hiding this comment.
The lambda captures variables by reference, but the lambda will be executed asynchronously in a thread pool. This creates dangling references to name and oflags. The original code correctly captured by value.
| internal::thread_pool_submit_reason::file_operation, [&] { | |
| internal::thread_pool_submit_reason::file_operation, [name, oflags] { |
| reactor::make_directory(std::string_view name_view, file_permissions permissions) noexcept { | ||
| auto name = sstring(name_view); | ||
| syscall_result<int> sr = co_await _thread_pool->submit<syscall_result<int>>( | ||
| internal::thread_pool_submit_reason::file_operation, [&] { |
There was a problem hiding this comment.
The lambda captures variables by reference, but the lambda will be executed asynchronously in a thread pool. This creates dangling references to name and permissions. The original code correctly captured by value.
| internal::thread_pool_submit_reason::file_operation, [&] { | |
| internal::thread_pool_submit_reason::file_operation, [name, permissions] { |
| reactor::touch_directory(std::string_view name_view, file_permissions permissions) noexcept { | ||
| auto name = sstring(name_view); | ||
| syscall_result<int> sr = co_await _thread_pool->submit<syscall_result<int>>( | ||
| internal::thread_pool_submit_reason::file_operation, [&] { |
There was a problem hiding this comment.
The lambda captures variables by reference, but the lambda will be executed asynchronously in a thread pool. This creates dangling references to name and permissions. The original code correctly captured by value.
| internal::thread_pool_submit_reason::file_operation, [&] { | |
| internal::thread_pool_submit_reason::file_operation, [name, permissions] { |
|
The problem is that coroutine allocation doesn't disable memory failure injection while allocating the coroutine frame (as we do when allocating a continuation). |
1 participant
Mostly improving readability.